Skip to content

refactor: extract _check_open_perms, fix dead elsif in sysopen#353

Draft
Koan-Bot wants to merge 1 commit intocpan-authors:mainfrom
atoomic:koan.atoomic/refactor-open-perms-check
Draft

refactor: extract _check_open_perms, fix dead elsif in sysopen#353
Koan-Bot wants to merge 1 commit intocpan-authors:mainfrom
atoomic:koan.atoomic/refactor-open-perms-check

Conversation

@Koan-Bot
Copy link
Copy Markdown
Contributor

@Koan-Bot Koan-Bot commented Apr 5, 2026

What

Extracts the permission-check block from __open, _io_file_mock_open, and __sysopen into a shared _check_open_perms() helper.

Why

The block was copy-pasted three times with subtle divergences. In __sysopen, the elsif branch (parent-dir write+execute check for new file creation) was dead code: the ENOENT guard above it ensured contents was always defined at that point. This meant sysopen(FH, $path, O_WRONLY|O_CREAT) in a restricted parent directory silently succeeded — while the equivalent open() correctly returned EACCES.

Closes #329 (dead elsif branch) and #330 (triple duplication).

How

  • Capture $is_new = !defined $mock_file->{'contents'} before O_CREAT populates contents
  • New _check_open_perms($mock_file, $abs_path, $rw, $is_new, $func_name, @args) replaces all three blocks
  • Normalizes autodie throw pattern: all paths now use _maybe_throw_autodie

Testing

  • All existing tests pass
  • New subtest in t/perms.t: validates sysopen O_CREAT respects parent-dir permissions

🤖 Generated with Claude Code


Quality Report

Changes: 9 files changed, 139 insertions(+), 1227 deletions(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

The open/sysopen permission-check block was copy-pasted three times
(in __open, _io_file_mock_open, and __sysopen) with subtle divergences.
In __sysopen, the elsif branch checking parent-dir perms was dead code:
contents was always defined at that point because the ENOENT check above
already returned. This meant sysopen(FH, $path, O_WRONLY|O_CREAT) in a
restricted parent directory silently succeeded — inconsistent with open().

Fix: capture $is_new before O_CREAT populates contents, extract a shared
_check_open_perms() helper used by all three call sites. Also normalizes
the autodie throw pattern (all now use _maybe_throw_autodie).

Closes cpan-authors#329
Closes cpan-authors#330

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Koan-Bot Koan-Bot force-pushed the koan.atoomic/refactor-open-perms-check branch from 87b2ab4 to abe174d Compare April 6, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Dead elsif branch in __sysopen permission check silently bypasses parent-dir write-permission enforcement

1 participant